feat(electrum): optimize merkle proof validation with batching #1957

LagginTimes · 2025-05-15T19:05:40Z

Replaces #1908, originally authored by @Keerthi421.
Fixes #1891.

Description

This PR optimizes sync/full_scan performance by batching and caching key RPC calls to slash network round-trips and eliminate redundant work.

Key improvements:

Gather all blockchain.transaction.get_merkle calls into a single batch_call request.
Use batch_script_get_history instead of many individual script_get_history calls.
Use batch_block_header to fetch all needed block headers in one call rather than repeatedly calling block_header.
Introduce a cache of transaction anchors to skip re-validating already confirmed transactions.

Anchor Caching Performance Improvements

Results suggest a significant speed up with a warmed up cache. Tested on local Electrum server with:

$ cargo bench -p bdk_electrum --bench test_sync

Results before this PR (https://github.com/LagginTimes/bdk/tree/1957-master-branch):

sync_with_electrum      time:   [1.3702 s 1.3732 s 1.3852 s]

Results after this PR:

sync_with_electrum      time:   [851.31 ms 853.26 ms 856.23 ms]

Batch Call Performance Improvements

No persisted data was carried over between runs, so each test started with cold caches and measured only raw batching performance. Tested withexample_electrum out of https://github.com/LagginTimes/bdk/tree/example_electrum_timing with the following parameters:

$ example_electrum init "tr([62f3f3af/86'/1'/0']tpubDD4Kse29e47rSP5paSuNPhWnGMcdEDAuiG42LEd5yaRDN2CFApWiLTAzxQSLS7MpvxrpxvRJBVcjhVPRk7gec4iWfwvLrEhns1LA4h7i3c2/0/*)#cn4sudyq"
$ example_electrum scan tcp://signet-electrumx.wakiyamap.dev:50001

Results before this PR:

FULL_SCAN TIME: 8.145874476s

Results after this PR (using this PR's bdk_electrum_client.rs):

FULL_SCAN TIME: 2.594050112s

Changelog notice

Add transaction anchor cache to prevent redundant network calls.
Batch Merkle proof, script history, and header requests.

Checklists

All Submissions:

I've signed all my commits
I followed the contribution guidelines
I ran cargo fmt and cargo clippy before committing

New Features:

I've added tests for the new feature
I've added docs for the new feature

Bugfixes:

This pull request breaks the existing API
I've added tests to reproduce the issue which are now passing
I'm linking the issue being fixed by this PR

evanlinjin

Thanks for moving this forward.

This is not a full review, but I think it's enough to push this PR in a good direction.

crates/electrum/src/bdk_electrum_client.rs

evanlinjin · 2025-05-16T11:22:06Z

crates/electrum/src/bdk_electrum_client.rs

+        // Batch validate all collected transactions.
+        if !txs_to_validate.is_empty() {
+            let proofs = self.batch_fetch_merkle_proofs(&txs_to_validate)?;
+            self.batch_validate_merkle_proofs(tx_update, proofs)?;
        }


Instead of having every populate_with_{} method call this internally, it will be more efficient and make more logical sense if we extract this so that we only call it at the end of full_scan and sync.

In other words, populate_with_{} should no longer fetch anchors. Instead, they should either mutate, or return a list of (Txid, BlockId) for which we try to fetch anchors for in a separate step.

It will be even better if full txs are fetched in a separate step too.

Partially resolved. This is the next TODO:

It will be even better if full txs are fetched in a separate step too.

~~This will likely be included in a separate PR.~~
Fetching all full txs in a batch call at the beginning of sync actually ended up doubling sync time.

@LagginTimes did you figure out why though?

crates/electrum/tests/test_electrum.rs

crates/electrum/src/bdk_electrum_client.rs

evanlinjin · 2025-05-26T01:39:19Z

@LagginTimes could you provide the benchmark results in the PR description and compare it to results before the changes in this PR?

notmandatory · 2025-05-26T16:24:01Z

Based on above benchmark results it looks like this change is 1s faster on sync, is that due to a small test size? Do we expect it to make more of a difference with wallets with many addresses?

evanlinjin · 2025-06-04T01:26:39Z

@LagginTimes ~~can you provide the code you used to test with a remote electrum server (instead of the testenv)?~~

Edit: how about we just test with the example-cli with a pre-populated signet wallet?

I suggested writing benchmarks with the assumption that local io (against testenv) would be slower than allocating memory (collecting requests into vec before batch requesting), however that assumption seems incorrect.

crates/electrum/src/bdk_electrum_client.rs

crates/electrum/benches/test_sync.rs

crates/electrum/src/bdk_electrum_client.rs

crates/electrum/benches/test_sync.rs

ValuedMammal · 2025-06-25T13:21:54Z

These are my criterion results after benching this PR. 👍

sync_with_electrum      time:   [37.325 ms 38.220 ms 38.805 ms]                              
                        change: [-85.756% -85.556% -85.308%] (p = 0.00 < 0.05)
                        Performance has improved.

ValuedMammal · 2025-06-25T13:23:15Z

In 838c247:

error: this file contains an unclosed delimiter
   --> crates/electrum/src/bdk_electrum_client.rs:724:3
    |
32  | impl<E: ElectrumApi> BdkElectrumClient<E> {
    |                                           - unclosed delimiter
...
504 |             for &(txid, height, hash) in chunk {
    |                                                - this delimiter might not be properly closed...
...
547 |         }
    |         - ...as it matches this but it has different indentation
...
724 | }

ValuedMammal

ACK 8086e1e

crates/electrum/src/bdk_electrum_client.rs

crates/electrum/benches/test_sync.rs

evanlinjin

Here are some suggestions to improve the benchmark.

I've implemented them in this commit: evanlinjin@1ba324c

I also added a benchmark for testing sync without cache. Feel free to cherry-pick this commit if you agree with the changes.

crates/electrum/benches/test_sync.rs

crates/electrum/src/bdk_electrum_client.rs

* Actually use different spks * Do not benchmark applying updates (only fetching/contructing) * Have two benches: One with cache, one without. * Remove `black_box`.

LagginTimes requested a review from evanlinjin May 15, 2025 19:06

LagginTimes self-assigned this May 15, 2025

evanlinjin requested changes May 16, 2025

View reviewed changes

LagginTimes marked this pull request as draft May 20, 2025 18:06

LagginTimes force-pushed the merkle_batching branch 2 times, most recently from d69907b to 149807c Compare May 21, 2025 18:42

evanlinjin reviewed May 23, 2025

View reviewed changes

crates/electrum/tests/test_electrum.rs Outdated Show resolved Hide resolved

crates/electrum/src/bdk_electrum_client.rs Outdated Show resolved Hide resolved

crates/electrum/src/bdk_electrum_client.rs Outdated Show resolved Hide resolved

notmandatory added the module-blockchain label May 23, 2025

notmandatory added this to BDK Chain May 23, 2025

notmandatory moved this to In Progress in BDK Chain May 23, 2025

LagginTimes force-pushed the merkle_batching branch 2 times, most recently from dc08959 to bf38a8e Compare May 25, 2025 18:30

LagginTimes marked this pull request as ready for review May 25, 2025 18:48

LagginTimes requested review from evanlinjin and ValuedMammal May 25, 2025 18:48

evanlinjin added this to the Wallet 2.0.0 milestone May 26, 2025

notmandatory modified the milestones: Wallet 2.0.0, Wallet 2.1.0 May 26, 2025

LagginTimes marked this pull request as draft May 26, 2025 23:18

LagginTimes force-pushed the merkle_batching branch from de14241 to bb26525 Compare May 27, 2025 18:16

LagginTimes force-pushed the merkle_batching branch 2 times, most recently from 90a0018 to 591b51a Compare June 8, 2025 17:32

jp1ac4 mentioned this pull request Jun 9, 2025

Bump bdk dependencies to latest wizardsardine/liana#1742

Open

LagginTimes marked this pull request as ready for review June 10, 2025 09:30

LagginTimes force-pushed the merkle_batching branch from 591b51a to 70495e2 Compare June 10, 2025 10:21

ValuedMammal reviewed Jun 10, 2025

View reviewed changes

crates/electrum/src/bdk_electrum_client.rs Outdated Show resolved Hide resolved

crates/electrum/src/bdk_electrum_client.rs Show resolved Hide resolved

ValuedMammal reviewed Jun 10, 2025

View reviewed changes

crates/electrum/src/bdk_electrum_client.rs Outdated Show resolved Hide resolved

fix(electrum): improve tx validation and gap limit scanning

b57768d

LagginTimes force-pushed the merkle_batching branch 2 times, most recently from 93f15e7 to 65e8a08 Compare June 20, 2025 21:33

LagginTimes requested a review from ValuedMammal June 23, 2025 19:37

ValuedMammal requested changes Jun 24, 2025

View reviewed changes

crates/electrum/benches/test_sync.rs Outdated Show resolved Hide resolved

crates/electrum/src/bdk_electrum_client.rs Outdated Show resolved Hide resolved

crates/electrum/src/bdk_electrum_client.rs Outdated Show resolved Hide resolved

ValuedMammal reviewed Jun 24, 2025

View reviewed changes

crates/electrum/benches/test_sync.rs Outdated Show resolved Hide resolved

LagginTimes force-pushed the merkle_batching branch from 65e8a08 to a7676a5 Compare June 24, 2025 23:55

ValuedMammal approved these changes Jun 25, 2025

View reviewed changes

LagginTimes force-pushed the merkle_batching branch 2 times, most recently from f12f2b2 to 8086e1e Compare June 25, 2025 18:40

LagginTimes requested a review from ValuedMammal June 25, 2025 19:01

ValuedMammal approved these changes Jun 25, 2025

View reviewed changes

evanlinjin reviewed Jun 25, 2025

View reviewed changes

crates/electrum/src/bdk_electrum_client.rs Outdated Show resolved Hide resolved

evanlinjin reviewed Jun 25, 2025

View reviewed changes

crates/electrum/src/bdk_electrum_client.rs Outdated Show resolved Hide resolved

evanlinjin reviewed Jun 25, 2025

View reviewed changes

crates/electrum/src/bdk_electrum_client.rs Outdated Show resolved Hide resolved

evanlinjin reviewed Jun 25, 2025

View reviewed changes

crates/electrum/src/bdk_electrum_client.rs Outdated Show resolved Hide resolved

evanlinjin reviewed Jun 25, 2025

View reviewed changes

crates/electrum/benches/test_sync.rs Outdated Show resolved Hide resolved

evanlinjin requested changes Jun 26, 2025

View reviewed changes

crates/electrum/benches/test_sync.rs Outdated Show resolved Hide resolved

crates/electrum/benches/test_sync.rs Show resolved Hide resolved

crates/electrum/benches/test_sync.rs Outdated Show resolved Hide resolved

evanlinjin reviewed Jun 26, 2025

View reviewed changes

crates/electrum/src/bdk_electrum_client.rs Show resolved Hide resolved

test(electrum): add criterion benchmark for sync

f21a21d

LagginTimes force-pushed the merkle_batching branch from 8086e1e to a94805a Compare June 26, 2025 17:34

LagginTimes requested a review from evanlinjin June 26, 2025 17:49

evanlinjin mentioned this pull request Jun 26, 2025

Add populate_anchor_cache method to BdkElectrumClient #1982

Open

5 tasks

evanlinjin requested changes Jun 27, 2025

View reviewed changes

crates/electrum/src/bdk_electrum_client.rs Outdated Show resolved Hide resolved

crates/electrum/src/bdk_electrum_client.rs Outdated Show resolved Hide resolved

crates/electrum/src/bdk_electrum_client.rs Outdated Show resolved Hide resolved

LagginTimes and others added 3 commits June 27, 2025 19:40

feat(electrum): batched Headers and script_get_history

ec4fd97

feat(electrum): batch transaction.get_merkle calls via batch_call

4ea5ea6

test(electrum): Improve benchmark

156cbab

* Actually use different spks * Do not benchmark applying updates (only fetching/contructing) * Have two benches: One with cache, one without. * Remove `black_box`.

LagginTimes force-pushed the merkle_batching branch from a94805a to 156cbab Compare June 27, 2025 19:40

feat(electrum): optimize merkle proof validation with batching #1957

Are you sure you want to change the base?

feat(electrum): optimize merkle proof validation with batching #1957

Uh oh!

Conversation

LagginTimes commented May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Anchor Caching Performance Improvements

Batch Call Performance Improvements

Changelog notice

Checklists

All Submissions:

New Features:

Bugfixes:

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

evanlinjin May 16, 2025

Choose a reason for hiding this comment

Uh oh!

LagginTimes May 21, 2025

Choose a reason for hiding this comment

Uh oh!

LagginTimes May 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

evanlinjin Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

evanlinjin commented May 26, 2025

Uh oh!

notmandatory commented May 26, 2025

Uh oh!

evanlinjin commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ValuedMammal commented Jun 25, 2025

Uh oh!

ValuedMammal commented Jun 25, 2025

Uh oh!

ValuedMammal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LagginTimes commented May 15, 2025 •

edited

Loading

LagginTimes May 25, 2025 •

edited

Loading

evanlinjin commented Jun 4, 2025 •

edited

Loading